Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Workspaces.MSBuild build with a netstandard target #76832

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

JoeRobich
Copy link
Member

Reworked #72257

Before this change we were building the Workspaces.MSBuild library (the part that loads in the end user's application process) as a .NET Core and .NET Framework library with no netstandard target, which meant that if we weren't careful we'd move our .NET Core TFM to something newer than what customers still expect us to support. All of our other libraries target netstandard but this one was still special. This was because some MSBuild NuGet packages themselves don't target netstandard and so we wre forced to do the same.

Digging further we realized that Microsoft.Build.Framework, which defines ILogger was already netstandard compatible, and so our only remaining use of an not-netstandard package was Microsoft.Build, which only existed to read solution files. That I fixed in our prior commit, so at this point the only NuGet packages we still referenced were .NET Standard compatible. Great!

There wa one more surprise though: the BuildHost we ship as content files in subdirectories, but we were also shipping the DLL as a regular referenced library in the end user's application. This was to provide the serialization exchange types to the RPC client, as well as share some useful helpers that were needed on both sides. But since the BuildHost still cannot target netstandard because it does need MSBuild libraries that are not yet netstandard, it meant that the regular Workspaces.MSBuild.dll project couldn't reference the BuildHost DLL anymore either. So to break that link I move the handful of files we were needing on both sides to a shared project, and then just include that shared project into both the build host and library/client projects. This means we can break the ProjectReference link entirely.

At some point MSBuild will make their other package netstandard, which means that split wasn't strictly necessary to do, but honestly it resulted in some downstream hacks so I believe it's a net win regardless. There was extra MSBuild/NuGet magic to make sure the binary was included in the other project without it appearing as a package reference. The only way to do that was to set PrivateAssets=all, which then meant other projects had to remember to reference that lest we fail to deploy a DLL. It was very much a fight against tooling, and severing the project references just cleans things up nicely.

Fixes #71784

We had a few different places doing fallback and this unifies it to
a single place.
MSBuildWorkspace had one version that would do the reading in-process
but did support solution filters. For VS Code we have one that was
doing it out-of-proces and didn't support solution filters. This
unifies both approaches, so everybody gets out of process and solution
filter support.

The solution filter code is simplified around exception handling:
there's a bunch of places it'd call TryGetAbsolute*Path asking for it
to throw if anything went wrong. We'd then catch that exception and
instead rethrow a generic "we couldn't read it" exception that also had
no information about the underlying failure. This just simplifies all
of that so we'll just let exceptions pass through.
Before this change we were building the Workspaces.MSBuild library (the
part that loads in the end user's application process) as a .NET Core
and .NET Framework library with no netstandard target, which meant that
if we weren't careful we'd move our .NET Core TFM to something newer
than what customers still expect us to support. All of our other
libraries target netstandard but this one was still special. This was
because some MSBuild NuGet packages themselves don't target netstandard
and so we wre forced to do the same.

Digging further we realized that Microsoft.Build.Framwork, which defines
ILogger was already netstandard compatible, and so our only remaining
use of an not-netstandard package was Microsoft.Build, which only
existed to read solution files. That I fixed in our prior commit, so
at this point the only NuGet packages we still referenced were
.NET Standard compatible. Great!

There wa one more surprise though: the BuildHost we ship as content
files in subdirectories, but we were also shipping the DLL as a regular
referenced library in the end user's application. This was to provide
the serialization exchange types to the RPC client, as well as share
some useful helpers that were needed on both sides. But since the
BuildHost still cannot target netstandard because it does need MSBuild
libraries that are not yet netstandard, it meant that the regular
Workspaces.MSBuild.dll project couldn't reference the BuildHost DLL
anymore either. So to break that link I move the handful of files we
were needing on both sides to a shared project, and then just include
that shared project into both the build host and library/client
projects. This means we can break the ProjectReference link entirely.

At some point MSBuild will make their other package netstandard, which
means that split wasn't strictly necessary to do, but honestly it
resulted in some downstream hacks so I believe it's a net win
regardless. There was extra MSBuild/NuGet magic to make sure the
binary was included in the other project without it appearing as a
package reference. The only way to do that was to set PrivateAssets=all,
which then meant other projects had to remember to reference that
lest we fail to deploy a DLL. It was very much a fight against tooling,
and severing the project references just cleans things up nicely.

Fixes #71784
@JoeRobich JoeRobich requested review from a team as code owners January 21, 2025 06:50
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 21, 2025
@JoeRobich JoeRobich changed the title Dev/jorobich/netstandard msbuildworkspace Make Workspaces.MSBuild build with a netstandard target Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenProjectAsync() is failing in docker container with base image of mcr.microsoft.com/dotnet/sdk:6.0
3 participants